Add numerically safe ordered probit distribution.#4232
Add numerically safe ordered probit distribution.#4232Spaak merged 9 commits intopymc-devs:masterfrom
Conversation
|
Hi @tohtsky thanks for the PR! This looks very useful indeed. As you can see some code checks are failing. Specifically the pre-commit one fails for some style reason (see the details there). If you fix that issue and push, the automatic tests will run again. We're currently in the process of lining up PRs for upcoming release 3.10, this one might still make it I think. |
Codecov Report
@@ Coverage Diff @@
## master #4232 +/- ##
==========================================
- Coverage 87.87% 87.85% -0.02%
==========================================
Files 88 88
Lines 14473 14501 +28
==========================================
+ Hits 12718 12740 +22
- Misses 1755 1761 +6
|
|
Hi @Spaak , thank you for the comment and sorry I'm not carefully reading the contribution guide. I have fixed the styling issues and made the test more accurate (the first test failed with float32 due to a numerical problem on the scipy side). I would be very happy to receive further comments and suggestions! |
Spaak
left a comment
There was a problem hiding this comment.
Actually, before merging, it would be great if you could add a line to the release notes, in the top, the "On deck" section.
|
Thanks for the review, @Spaak ! |
|
Thanks again @tohtsky! 🎉 |
This feature adds a ordered probit distribution,
which uses probit link function (i.e., std_cdf) instead of logistic one (sigmoid) used by OrdredLogistic.
Since std_cdf converges to 1 or 0 much faster than sigmoid, additional care must be taken
in order to deal with outlier values without encountering numerical instability,
For example, if we simply use std_cdf instead of sigmoid in the implementation of
OrderedLogistic,we will be getting
bad initial energyerror in the following example.The implemented version of
OrderedProbitavoids this issue by carefully using scaled error functionerfcx.